[#5626] feat (gvfs-fuse): Set up a submodule project for the Gvfs-fuse #5627
Conversation
clients/gvfs-fuse/build.gradle.kts
Outdated
There was a problem hiding this comment.
I think you need use a Gradle for Rust plugin to compile this module, just like client-python.
maybe we can use the Gradle Rust plugin to install Rust compile tools in the temp dir.
There was a problem hiding this comment.
I tried some rust plugins. I can't found a good one
|
suggest to do some auto check like |
clients/gvfs-fuse/check_rust_env.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| export PATH="$HOME/.cargo/bin:$PATH" |
There was a problem hiding this comment.
What if I run this script several times?
There was a problem hiding this comment.
If successful, the install branch can be skipped
There was a problem hiding this comment.
I'd suggest we don't do this in the script.
Most of the software installer scripts avoid doing this.
It may and may not be desirable for the user.
| if ! command -v cargo &> /dev/null; then | ||
| echo "Rust is not installed. Installing Rust..." | ||
|
|
||
| if command -v curl &> /dev/null; then | ||
| curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y | ||
| elif command -v wget &> /dev/null; then | ||
| wget -qO- https://sh.rustup.rs | sh -s -- -y | ||
| else | ||
| echo "Error: Neither curl nor wget is available. Please install one of them to proceed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| export PATH="$HOME/.cargo/bin:$PATH" | ||
| if command -v cargo &> /dev/null; then | ||
| echo "Rust has been installed successfully." | ||
| else | ||
| echo "Error: Rust installation failed. Please check your setup." | ||
| exit 1 | ||
| fi | ||
| else | ||
| echo "Rust is already installed: $(cargo --version)" | ||
| fi |
There was a problem hiding this comment.
I'd do this to 1) return early if possible 2) save some of the nested if- tests.
if command -v cargo &> /dev/null; then
echo "Rust is already installed: $(cargo --version)"
exit 0
fi
echo "Rust is not installed. Installing Rust..."
# the rest of the install and check
# ...
if ! command -v cargo &> /dev/null; then
echo "Error: Rust installation failed. Please check your setup."
exit 1
fi
echo "Rust has been installed successfully."
exit 0There was a problem hiding this comment.
The cargo command does not run after running the rust installation command. Unless you set the PATH environment variable or restart the bash session.
There was a problem hiding this comment.
well ... we are assuming that cargo is installed in a specific directory, i.e. $HOME/.cargo/bin, any way...
The only difference is that we manually invoke the binary (with full path) or let the "polluted" shell to find it. Right?
There was a problem hiding this comment.
Yes, the cargo install script will add logic to modify the PATH inn the .profile. If we restart the shell session, it will automatically add this environment variable.
Is modifying the PATH not allowed?
|
@FANNG1 can you please help to review? |
| [package] | ||
| name = "filesystem-fuse" | ||
| version = "0.8.0-incubating-SNAPSHOT" | ||
| edition = "2021" |
There was a problem hiding this comment.
What is the usage of this "edition", shall we remove this?
There was a problem hiding this comment.
The version specify the minimum Rust compiler version required for this project.
|
suggest adding basic check for code style and common errors in ci |
| name = "filesystem-fuse" | ||
| version = "0.8.0-incubating-SNAPSHOT" | ||
| edition = "2021" | ||
| rust-version = "1.75" |
There was a problem hiding this comment.
also add homepage, repository, license ?
There was a problem hiding this comment.
@diqiu50 do you plan to add these things?
clients/filesystem-fuse/Cargo.toml
Outdated
| path = "src/main.rs" | ||
|
|
||
| [dependencies] | ||
| fuse3 = { version = "0.8.1", "features" = ["tokio-runtime", "unprivileged"] } |
There was a problem hiding this comment.
There are many fuse implement in RUST, why you choose fuse3?
There was a problem hiding this comment.
seems fuser is more popular
There was a problem hiding this comment.
Yes, fuser is more popular, but FUSE3 is used by OpenDAL. We need to decide which one to use soon
|
@FANNG1 can you please help to review again? If there's no blocking issue, I think we can improve the building command in the following PRs, what do you think? |
Seems there're some basic issues to be addressed,
|
|
@FANNG1 |
| name = "filesystem-fuse" | ||
| version = "0.8.0-incubating-SNAPSHOT" | ||
| edition = "2021" | ||
| rust-version = "1.75" |
There was a problem hiding this comment.
@diqiu50 do you plan to add these things?
|
|
||
| set -e | ||
|
|
||
| if ! command -v cargo &> /dev/null; then |
There was a problem hiding this comment.
rename the file to install_rust_env.sh, and place it in dev directory?
|
LGTM, except minor comments |
What changes were proposed in this pull request?
Set up a submodule project for the Gvfs-fuse, layout the rust module environment for gvfs-fuse
Why are the changes needed?
Fix: #5626
Does this PR introduce any user-facing change?
No
How was this patch tested?
No